[TS] Fix generation of struct members in object api #7148

tira-misu · 2022-03-07T07:07:09Z

If a struct is used in a struct TS generates wrong code.
Example:

struct A {
testBool: bool;
}

struct B {
teststruct: A;
}

Furthermore:

null value for type bool was wrong ("false" instead of "0")
fixed reserved word argument -> arguments
field accessor function must be unescaped

Pull from google/flatbuffers

If a struct has a key the vector has to be sorted. To sort the vector you can't use "const".

…o master

* option indent_step is supported

…o master

CasperN · 2022-03-07T16:54:37Z

Hello, I'm not the TS maintainer but I am working on standardizing our code generation and would like you opinion on #7142. I'm curious why "field accessor function must be unescaped" does TS/JS first resolve fields before trying to find keywords?

I noticed you only changed idl_gen_ts. I think you should make a test schema that demonstrates your changes. I think adding ts generation to keyword_test.fbs should do it.

Also, pull again - I managed to break master when adding a printf and I fixed that.

tira-misu · 2022-03-07T17:08:41Z

About reserved word as functions in TS. Yes - its a bit strange, but indeed reserved words are allowed as function/method names. In other languages that's not the case. So TS seams to be a special snowflake here. But in my opinion its more convenient for a user of TS to not escape these method names.

I will add a test case to the fbs file and try to run the test.

CasperN · 2022-03-16T03:16:22Z

@krojew can we get a review?

krojew · 2022-03-16T05:49:19Z

tests/my-game/example/struct-of-structs.ts

-    (this.b === null ? 0 : this.b.b!),
-    (this.c === null ? 0 : this.c.id!),
-    (this.c === null ? 0 : this.c.distance!)
+    (this.a === null ? 0 : this.a?.id!),


This seems quite odd - you first allow a to be undefined (?), but then you assert the value is present (!). Looks contradictory. The same pattern repeats for other fields in this PR.

Of cause this looks a bit strange. We could simplify this line from (this.a === null ? 0 : this.a?.id!) to (this.a?.id!). It is equivalent and should compile.

This would still be wrong - the result of the ?. operator can be undefined, so you can't later assert it's not by using !.

What would you suggest?
Would this solution be ok? :

struct of struct: (this.a === null ? 0 : this.a.id!)

struct of struct of struct: (this.a?.b === null ? 0 : this.a.b.id!) <- i think this will not compile

Why do you need ?. in the first place? There is already a null check present.

Ok, I see what the problem might be for nested structs, but the proposed code doesn't solve it. For example, this.a?.a === null will never be true if this.a is null, thus defeating the check. Instead, something like this should work: this.a?.a ?? 0.

You are right. The solution is:

struct of struct: this.a?.id ?? <nullvalue> (wrong: this.a?.id && <nullvalue>)

struct of struct of struct: this.a?.b?.id ?? <nullvalue> (wrong: this.a?.b?.id && <nullvalue>=

is false for type bool and 0 for all other simple data types.
I will try it out. If it works i will alter this pull request. I will also write a test case for this.

No, this is not what I proposed - using && instead of ?? will not work.

Of cause - a typo at my side. Thanks

dbaileychess · 2022-03-23T04:42:47Z

Are we all good to go on this?

tira-misu · 2022-03-23T04:48:43Z

From my point of view yes.

dbaileychess · 2022-03-24T04:40:19Z

Thanks as always!

tira-misu and others added 19 commits October 1, 2019 07:51

Merge pull request #2 from google/master

64639a2

Pull from google/flatbuffers

Fix C/C++ Create<Type>Direct with sorted vectors

1d907c5

If a struct has a key the vector has to be sorted. To sort the vector you can't use "const".

Changes due to code review

7827687

Improve code readability

a78916b

Merge branch 'master' of https://github.com/tira-misu/flatbuffers int…

0d9e229

…o master

Add generate of JSON schema to string to lib

163d43b

* option indent_step is supported

Remove unused variables

2793d38

Fix break in test

79cd673

Fix style to be consistent with rest of the code

6085fe1

Merge branch 'master' of https://github.com/tira-misu/flatbuffers int…

7ae0e9d

…o master

Merge remote-tracking branch 'upstream/master' into master

0219f66

Merge branch 'master' of https://github.com/google/flatbuffers

8643e11

Merge branch 'google:master' into master

b837543

[TS] Fix reserved words as arguments (google#6955)

eff8031

Merge remote-tracking branch 'github/master'

3aa4f37

[TS] Fix generation of reserved words in object api (google#7106)

fd6a5bb

[TS] Fix generation of object api

cfd969e

Merge branch 'master' of https://github.com/google/flatbuffers

d37c5cc

[TS] Fix MakeCamel -> ConvertCase

40a0b4f

github-actions bot added c++ codegen Involving generating code from schema javascript typescript labels Mar 7, 2022

CasperN mentioned this pull request Mar 7, 2022

RFC: Centralize naming convention #7142

Closed

19 tasks

tira-misu added 3 commits March 9, 2022 06:34

Merge remote-tracking branch 'github/master'

12d352d

[TS] Add test for struct of struct of struct

07f65e4

Update generated files

3eb6b78

github-actions bot added the dart label Mar 9, 2022

github-actions bot added json lobster python rust swift labels Mar 9, 2022

Add missing files

f728905

github-actions bot added c# golang java kotlin lua php labels Mar 9, 2022

krojew reviewed Mar 16, 2022

View reviewed changes

[TS] Fix query of null/undefined fields in object api

7aaaedd

krojew approved these changes Mar 23, 2022

View reviewed changes

dbaileychess merged commit 2ad4086 into google:master Mar 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TS] Fix generation of struct members in object api #7148

[TS] Fix generation of struct members in object api #7148

tira-misu commented Mar 7, 2022

CasperN commented Mar 7, 2022

tira-misu commented Mar 7, 2022

CasperN commented Mar 16, 2022

krojew Mar 16, 2022

tira-misu Mar 16, 2022

krojew Mar 16, 2022

tira-misu Mar 16, 2022

krojew Mar 16, 2022

krojew Mar 16, 2022

tira-misu Mar 16, 2022 •

edited

Loading

krojew Mar 16, 2022

tira-misu Mar 16, 2022

dbaileychess commented Mar 23, 2022

tira-misu commented Mar 23, 2022

dbaileychess commented Mar 24, 2022

[TS] Fix generation of struct members in object api #7148

[TS] Fix generation of struct members in object api #7148

Conversation

tira-misu commented Mar 7, 2022

CasperN commented Mar 7, 2022

tira-misu commented Mar 7, 2022

CasperN commented Mar 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tira-misu Mar 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbaileychess commented Mar 23, 2022

tira-misu commented Mar 23, 2022

dbaileychess commented Mar 24, 2022

tira-misu Mar 16, 2022 •

edited

Loading